Skip to content

test: cover sqlite statements after database close#62978

Closed
umuoy1 wants to merge 2 commits into
nodejs:mainfrom
umuoy1:test-sqlite-close-invalidates-statements
Closed

test: cover sqlite statements after database close#62978
umuoy1 wants to merge 2 commits into
nodejs:mainfrom
umuoy1:test-sqlite-close-invalidates-statements

Conversation

@umuoy1
Copy link
Copy Markdown
Contributor

@umuoy1 umuoy1 commented Apr 26, 2026

This PR adds coverage for DatabaseSync closing behavior with existing prepared statements.

The tests verify that after close() or [Symbol.dispose]():

  • database methods such as prepare() and exec() report that the database is not open
  • existing StatementSync instances report that the statement has been finalized
  • reopening the database does not make old statements usable again

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Apr 26, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.05%. Comparing base (131fd84) to head (ad0690e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62978      +/-   ##
==========================================
- Coverage   91.94%   90.05%   -1.89%     
==========================================
  Files         362      714     +352     
  Lines      155999   225242   +69243     
  Branches    24057    42582   +18525     
==========================================
+ Hits       143429   202849   +59420     
- Misses      12295    14187    +1892     
- Partials      275     8206    +7931     

see 477 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@umuoy1
Copy link
Copy Markdown
Contributor Author

umuoy1 commented May 13, 2026

Hi, just wanted to check if someone from @nodejs/sqlite could take a look at this when they have time.

This is a test-only PR for node:sqlite, adding coverage for StatementSync and iterators after DatabaseSync.close() / Symbol.dispose. If this looks reasonable, could someone also help start CI?

Thanks!

@umuoy1 umuoy1 force-pushed the test-sqlite-close-invalidates-statements branch from 81d2d8f to ad0690e Compare May 13, 2026 07:35
@louwers
Copy link
Copy Markdown
Contributor

louwers commented May 13, 2026

I think there's already two other PRs that implement this as well.

@umuoy1 umuoy1 closed this May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants